Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix tests broken by add_root change #29248

Merged
merged 1 commit into from
Dec 13, 2022
Merged

fix tests broken by add_root change #29248

merged 1 commit into from
Dec 13, 2022

Conversation

jeffwashington
Copy link
Contributor

@jeffwashington jeffwashington commented Dec 13, 2022 •
edited
Loading

Problem

somehow these 2 tests are broken.
test_handle_dropped_roots_for_ancient
test_clean_and_unclean_slot

Summary of Changes

add to uncleaned_roots correctly after add_root calls.

Fixes #

@apfitzge
Copy link
Contributor

For clarity, the change that broke these tests was #29245?

apfitzge
apfitzge approved these changes Dec 13, 2022
View reviewed changes
Copy link
Contributor

@apfitzge apfitzge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change looks fine to me; let's wait for the CI so we make sure we didn't miss asserts on the uncleaned_roots in any other tests

@jeffwashington jeffwashington merged commit 4ae12bf into solana-labs:master Dec 13, 2022
gnapoli23 pushed a commit to gnapoli23/solana that referenced this pull request Dec 16, 2022
@jeffwashington @gnapoli23
fix tests broken by add_root change (solana-labs#29248)
e06a21a
nickfrosty pushed a commit to nickfrosty/solana that referenced this pull request Jan 4, 2023
@jeffwashington @nickfrosty
fix tests broken by add_root change (solana-labs#29248)
0db225c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@apfitzge apfitzge apfitzge approved these changes

@brooksprumo brooksprumo Awaiting requested review from brooksprumo

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jeffwashington @apfitzge